perf(V2): bound transcript memory - #648
Conversation
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review, pass 1: changes required
Merge is blocked pending correction of these findings:
- Client replacement currently emits full instance-destruction semantics, which can discard active selection, drafts, expansion state, and transcript state; pending permission/question synchronization can also apply stale responses from the replaced client.
- Subagent transcript loading can start before the child session exists and does not safely share request ownership across multiple mounted consumers.
- Running native refresh/session-status continuations can survive instance teardown and recreate or mutate state after removal.
- Permission resources, task output, todo output, and apply-patch copy paths can hide or make authoritative content inaccessible while still allowing user decisions.
- Retained-size accounting misses Map/Set contents and can double-count shared ArrayBuffer storage.
- Info-view back navigation, same-diff permission identity, search partiality/revealability, descendant totals, and exact task-limit signaling need correction.
I am applying the corrections now and will publish another gatekeeper pass after validation. The PR remains Draft.
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review, pass 2: changes still required
The first-pass findings are corrected and the combined branch passes UI typecheck, production build, and 71 focused tests. A second full review found additional blockers:
- Streaming mutations can repeatedly cancel retained-size measurement, leaving active growth unaccounted; LRU touches before accounting are also lost.
- Client/reconnect invalidation must clear old load errors, force authoritative reconciliation, and invalidate request sync even when the shared V2 proxy/client identity is unchanged.
- Shared-load lease completion can overwrite newer authority, and mounted child tasks do not react to loaded-state invalidation.
- Permission keyboard shortcuts can approve while another button is focused; event/reply continuations need current instance/client/request authority.
- Large permission review needs bounded processing and a non-clipboard access path; the modal also needs keyboard focus containment.
- Search must continue after marking a truncated part partial; aggregate-truncated diffs need fallback detection; truncated child steps and Status-tab todos need complete-content access.
- Child deletion must recompute ancestor totals, Info restore must retain its conversation target, and the new runnable tests must be added to CI.
I am applying pass-2 corrections now. The PR remains Draft and blocked until a subsequent pass reports no findings.
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review, pass 3: changes still required
Pass-2 corrections now pass full UI/Electron typecheck, 28 standard focused tests, and 63 browser-condition tests. The third full review found remaining interactions:
- External forced transcript reloads can trigger the mounted loader and abort the caller-owned refresh; reload ownership must be single-source.
- Evicting an infinite retained-size estimate can poison LRU arithmetic with NaN.
- Long sessions must follow native V2 message pagination so eviction/reload restores complete history.
- Same-client reconnect authority is still missing from session-info and question reply continuations; permission event merging/reply completion also needs stable request semantics.
- Search indexing must match actually rendered task/apply-patch/output regions and budgets.
- Measurement and async Markdown/diff cache work must not resurrect stale data after mutation/eviction.
- The CI cleanup correctly removes Tauri, but must retain the independent Windows server spawn test.
- Permission shortcuts inside the active modal must remain usable without duplicate document handling.
- Tool-call documentation still describes the old truncation/copy behavior.
I am applying pass-3 corrections and will repeat the gatekeeper review. The PR remains Draft and blocked.
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review, pass 4: changes still required
Pass-3 corrections typecheck successfully and fix full V2 message pagination, cache resurrection, LRU Infinity handling, reconnect authority, search alignment, and Windows Node validation. Pass 4 found remaining blockers:
- Authoritative message replacement must purge omitted message render caches; failed/empty accounting must not retain pending LRU touches.
- Tool search/render parity still has gaps for fallback output, diagnostics, collection budgets, task truncation, and input copy format.
- Shared pending-request synchronization needs consumer leases so one timeout cannot cancel reconnect recovery; several same-client reconnect paths still need authority/version checks and delta-buffer cleanup.
- The standard CI partition needs forced exit for tests that retain UI handles.
- Removing Tauri setup from the caller is insufficient while the reusable build workflow still enables Tauri jobs; the DEV-v2 PR path must disable those jobs while retaining the Windows Node spawn test.
- Migration documentation must retain unresolved security work not addressed by this UI-only PR and describe Tauri removal narrowly and accurately.
I am applying pass-4 corrections now and will run another full gatekeeper pass. The PR remains Draft and blocked.
Limit rendered tool output, diffs, diagnostics, search results, and nested task summaries so large OpenCode transcripts do not create unbounded Markdown, ANSI, syntax-highlighted, or diff DOM trees. Keep complete authoritative values in the V2 message store and resolve full copy payloads only after explicit user action. Oversized permission diffs require successful full-copy access before approval while rejection remains available. Bound derived render caches and add focused coverage for rendering limits, lazy full-content copy, diagnostics, nested tasks, and partial search results. UI typecheck, 22 focused tests, and git diff --check pass.
Apply one 64 MiB byte-aware LRU budget across root and subagent transcripts in every workspace. Whole-session eviction preserves scroll and clears normalized plus derived caches, while later selection reloads complete history through the native OpenCode V2 message API. Protect visible, loading, working, compacting, generation-pending, streaming, optimistic-send, permission-blocked, and question-blocked sessions. Keep only the active session view mounted and measure retained data incrementally so budget accounting does not stall the renderer. Focused memory, message-store, permission, send-lifecycle, and native reload tests pass together with the UI typecheck and git diff check.
Abort in-flight transcript loads when their view, session, instance, or client authority disappears, and reject late responses from clients that ignore cancellation. Load subagent transcripts only while their task view is mounted, purge per-instance session state and native refresh timers on teardown, and stop eagerly hydrating descendant transcripts with root sessions. Also remove reverted permission and question queue entries so hidden runtime state cannot retain obsolete message data. Covered by focused request-authority, native-event, hook, message-store, and transcript-memory tests.
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review, post-rebase pass: changes still required
The branch is now rebased onto DEV-v2@c66aac79, which removed the duplicate Rust-native event transport and resolved the prior server/service findings. Redundant reconnect buffering and Tauri workflow changes were removed. Current validation passes full typecheck plus 140 focused tests.
The remaining findings are limited to this PR's renderer-memory scope:
- Per-session derived render/display caches must be included in transcript accounting now that the old five-session cap is gone.
- Structured input and �pply_patch still perform a complete eager formatting/join before truncation or copy.
- Search parity has two remaining empty-diff/error-budget gaps.
- Long single task titles need a complete copy path; diagnostic dedupe, empty copy buttons, and live-region review semantics need small corrections.
I am applying this final scoped correction set and will repeat the review. The PR remains Draft.
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review, final scoped pass: two axes clear, corrections remain
The rebased branch now has no async-authority finding and no CI/documentation finding. DEV-v2 authorization is included, upstream Tauri validation is preserved, and the current focused suites/typecheck pass.
Remaining renderer-memory findings:
- The permission modal portal must close with its owning hidden instance, and complete permission reviews must not mount multi-megabyte content while collapsed.
- Permission title/resource checks, todo normalization, apply-patch file selection, and transcript measurement still create some eager full-collection snapshots.
- Derived display caches and unknown pending parts need complete accounting/cleanup, and touching an inactive transcript must refresh accounting after render-cache growth.
- Empty child-task scan windows must still expose truncation/full-copy, and search limits must match actually renderable structured output/prompt regions.
I am applying this final renderer/memory correction set. Async and CI/docs gates remain clear; the PR stays Draft until the next pass reports no findings.
Re-scope the renderer-memory fix on the native V2 baseline after removal of the duplicate Tauri event transport. Bound Markdown, reasoning, tool output, diagnostics, diffs, task steps, todos, and per-message part rendering while retaining complete authoritative content behind lazy copy actions. Coordinate message records and derived render caches under one 64 MiB byte-aware LRU. Protect mounted, loading, and live transcripts; cap pending parts and prompt display overrides; fence stale cache writes; and conservatively account measurement failures. Reload evicted transcripts through bounded complete V2 pagination, reject stale lifecycle completions, purge all per-instance session state, and prevent delayed SSE/delta work from recreating removed stores. Keep DEV-v2 PR validation enabled without reintroducing native Tauri transport changes. Validated with full UI and Electron typecheck, UI production build, 248 standard UI tests, 71 browser-condition UI tests, 118 Electron native tests, and git diff checks. The long-duration WebKit soak remains pending while the PR stays draft.
2910e88 to
bb39dab
Compare
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review: scoped implementation clear
The branch is now rebased onto DEV-v2@c66aac79 and the duplicate Rust-native Tauri event transport remains removed. The PR was re-scoped to the renderer-memory issue: bounded DOM/eager processing, one global transcript/cache budget, lossless bounded V2 reload, and complete teardown cleanup. Search and permission-policy expansions were removed.
Final validation passes full UI/Electron typecheck, production UI build, 248 standard UI tests, 71 browser-condition UI tests, 118 Electron native tests, and diff checks. The final child-session discovery race was corrected and its focused tests pass.
No scoped gatekeeper finding remains. The PR stays Draft only for the outstanding 24-hour WebKit renderer-memory soak.
Integrate the current native V2 runtime, inline form flow, pruned validation matrix, and shared desktop/runtime fixes without restoring the removed delta-buffer transport. Keep PR 648's bounded transcript rendering and inactive-memory budget, then replace oversized transcript rejection with stale-safe 200-message window paging. Consolidate duplicate caches, account native and pending state, preserve paging cursors through scroll snapshots, and keep authoritative active content untruncated. Reduce feature-specific tests to representative memory, eviction, rendering, lazy-copy, paging, authority, and cleanup invariants. UI typecheck and build pass; focused standard/browser suites, the server suite, Electron native suite, and diff checks pass.
…60818 # Conflicts: # packages/ui/src/stores/message-v2/instance-store.ts # packages/ui/src/stores/message-v2/types.ts # packages/ui/src/stores/native-session-streaming.test.ts # packages/ui/src/stores/native-session-streaming.ts # packages/ui/src/stores/session-api.ts # packages/ui/src/stores/session-events.ts
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32199549214 Artifacts expire in 7 days.
|
1 similar comment
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32199549214 Artifacts expire in 7 days.
|
…60818 # Conflicts: # packages/ui/src/stores/session-request-authority.test.ts
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32202206283 Artifacts expire in 7 days.
|
1 similar comment
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32202206283 Artifacts expire in 7 days.
|
Stop eager descendant transcript loads, keep live events out of historical paging windows, protect Form-blocked sessions, and release the SDK live reducer after terminal events or transcript eviction. Derived caches now evict globally oldest entries instead of flushing every session. Require successful access to the full patch before approving a truncated permission diff. Add regressions for historical/live composition, permission gating, Form protection, and oldest-first cache eviction; validate root typechecks, 15 focused tests, UI build, and diff cleanliness.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32215067376 Artifacts expire in 7 days.
|
1 similar comment
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32215067376 Artifacts expire in 7 days.
|
Propagate full-diff review state to keyboard shortcuts and disable unresolved fallback approval actions for truncated patches. Rejection remains available, while approval requires a successful full-copy access through the rendered diff. Validated with root typechecks, the permission safety regression, and git diff --check.
Scope native createData reducers per session so terminal events and transcript eviction cannot discard another active session's streaming state. Remove replaced cache entries before global enforcement to keep retained-byte and entry counters exact. Add regressions for cross-session terminal isolation and replacement under cache pressure; validate root typechecks and focused tests.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32226248908 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32228465460 Artifacts expire in 7 days.
|
1 similar comment
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32228465460 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32257947226 Artifacts expire in 7 days.
|
Migrate the Status panel from interactive PTYs to the location-scoped shell API, refresh shell state after lifecycle events and reconnects, and route locationless shell events by their owned working directory. Allowlist shell routes explicitly and verify ShellInfo.cwd before every ID-scoped request, including trailing-slash aliases, while preserving native output cursor parameters. Keep PTYs separate for interactive terminal use. Load project metadata before the first session inventory, preserve native projected message order, invalidate inactive transcripts instead of reloading every transcript after reconnect, and keep native shutdown terminal without latching ordinary navigation flushes. Update V2 documentation and CI test paths. Validated with UI and server typechecks, 15 focused UI tests, 35 focused server tests, and git diff --check.
# Conflicts: # packages/ui/src/stores/opencode-data.test.ts # packages/ui/src/stores/opencode-data.ts
Summary
This supersedes #626 without modifying or closing that historical PR and closes #553.
Updated V2 Base
Merged
DEV-v2@e1987b9c; PR HEAD is87bd4304(3,111+/693-, 85 files).@opencode-ai/client/solidcreateDatawithout replacing REST-loaded history; each internal reconnect generation clears stale live projection state before authoritative reload0.0.0-beta-17595; the independently updated CLI is validated through service/API compatibility without an exact version gateValidation
15/15; inherited server composition18/18199/199 + 47/47, Electron122/122, server255passed with3platform skipscargo check --locked, andgit diff --checkRemaining